-
Notifications
You must be signed in to change notification settings - Fork 105
Conversation
If 0 length is passed in, we should return math.NaN like python code Also follow SEP (Somebody else's problem)
8dcab6c
to
29b65f6
Compare
Basically ported over https://github.com/grafana/metrictank/blob/master/expr/seriesaggregators.go ``` median def safeMedian(values): safeValues = [v for v in values if v is not None] if safeValues: sortedVals = sorted(safeValues) mid = len(sortedVals) // 2 if len(sortedVals) % 2 == 0: return float(sortedVals[mid-1] + sortedVals[mid]) / 2 else: return sortedVals[mid] diff def safeDiff(values): safeValues = [v for v in values if v is not None] if safeValues: values = list(map(lambda x: x*-1, safeValues[1:])) values.insert(0, safeValues[0]) return sum(values) stddev def safeStdDev(a): sm = safeSum(a) ln = safeLen(a) avg = safeDiv(sm,ln) if avg is None: return None sum = 0 safeValues = [v for v in a if v is not None] for val in safeValues: sum = sum + (val - avg) * (val - avg) return math.sqrt(sum/ln) range 'range': lambda row: safeSubtract(safeMax(row), safeMin(row)), multiply 'multiply': lambda row: safeMul(*row), def safeMul(*factors): if None in factors: return None factors = [float(x) for x in factors] product = reduce(lambda x,y: x*y, factors) return product ```
Note that this fixes bug --> unable to get cnt aggfunc Also ignores archive which seems to be different
29b65f6
to
5136160
Compare
batch/aggregator.go
Outdated
func Mult(in []schema.Point) float64 { | ||
valid := false | ||
mult := float64(1) | ||
for _, term := range in { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
term is the terminology for sum, when it's multiplication they're called factors.
batch/aggregator.go
Outdated
for _, term := range in { | ||
if math.IsNaN(term.Val) { | ||
// NaN * anything equals NaN() | ||
mult = math.NaN() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just return term.Val
here and skip all the other stuff
batch/aggregator.go
Outdated
avg := Avg(in) | ||
if !math.IsNaN(avg) { | ||
num := float64(0) | ||
totalDeviationSquared := float64(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
confusing name. it's not one total deviation that is squared. it's the sum of squares of deviations.
batch/aggregator.go
Outdated
|
||
func StdDev(in []schema.Point) float64 { | ||
avg := Avg(in) | ||
if !math.IsNaN(avg) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could turn this into an early return and reduce the needed indentation
batch/aggregator.go
Outdated
func Range(in []schema.Point) float64 { | ||
min := Min(in) | ||
max := Max(in) | ||
return max - min |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can speed this function up by only looping once
a417199
to
3e0b54f
Compare
3e0b54f
to
d57d220
Compare
LGTM. thanks @Spellchaser |
This is also in support of the summarize function #837
Basically ported over https://github.com/grafana/metrictank/blob/master/expr/seriesaggregators.go
Also removed panics since they all return math.NaN